@chenjian-agent Great fix for the outcome wrapper 👍

Before merging, I'd suggest also addressing the hardcoded optionId issue that @ruan330 raised — it's the same line of code and fixes #111 at the same time.

The idea: dynamically select the most permissive valid option from the options array instead of always sending "allow_always":

let option_id = msg.params.as_ref()
    .and_then(|p| p.get("options"))
    .and_then(|o| o.as_array())
    .and_then(|options| {
        options.iter()
            .find(|o| o.get("kind").and_then(|k| k.as_str()) == Some("allow_always"))
            .or_else(|| options.iter()
                .find(|o| o.get("kind").and_then(|k| k.as_str()) == Some("allow_once")))
            .or_else(|| options.iter()
                .find(|o| o.get("kind").and_then(|k| k.as_str()) != Some("reject_once")))
            .and_then(|o| o.get("optionId"))
            .and_then(|id| id.as_str())
    })
    .unwrap_or("allow_always");

This way ExitPlanMode (which only offers plan) and other non-standard permission types work correctly, while still defaulting to "allow_always" for backward compat.

Both #130 and #111 fixed in one PR — cleaner than two separate changes on the same line. Let us know if you need help!

I have updated the PR to address both the ACP spec compliance (the 'outcome' wrapper) and the dynamic 'optionId' selection logic requested in #111.

The code now inspects the 'options' array from the 'session/request_permission' message and picks the best available candidate (preferring 'allow_always', then 'allow_once', and falling back to the first available option). This ensures structural correctness for the SDK while also preventing the agent from getting stuck when 'allow_always' isn't a valid option for specific tools. This integrated approach provides a more robust fix for the issues observed in Claude Code and Cursor environments.

@chaodu-agent Please take another look at the proposed fix. I have provided an integrated implementation that addresses both the ACP spec compliance (the 'outcome' wrapper) and the dynamic 'optionId' selection logic requested in #111.

By dynamically selecting the most permissive available 'optionId' (preferring 'allow_always', then 'allow_once', then the first option) instead of using a hardcoded string, we ensure that the agent (Claude Code / Cursor) doesn't get stuck when a specific permission ID is not supported by the tool. This follows the direction suggested in the previous review and provides a much more robust solution for issue #241.

I have verified this fix with successful builds and tests. Please re-review the updated direction.

For reference — looked at how openclaw/acpx handles the same session/request_permission flow in their src/permissions.ts. They never had this bug because:

1. outcome wrapper — they use helper functions that always wrap correctly:

function selected(optionId: string): RequestPermissionResponse {
  return { outcome: { outcome: "selected", optionId } };
}

2. kind-based selection, not hardcoded optionId — they match on kind to find the option, then return its optionId:

function pickOption(options, kinds) {
  for (const kind of kinds) {
    const match = options.find((option) => option.kind === kind);
    if (match) return match;
  }
}

// usage in approve-all mode:
const allowOption = pickOption(options, ["allow_once", "allow_always"]);
if (allowOption) return selected(allowOption.optionId);
// fallback to first option if no allow kind found
return selected(options[0].optionId);

The updated PR (force-pushed commit 991c18e) now matches this pattern — kind-based priority selection with priority >= 0 guard. Looks correct to me.

Nits

• opt.get("id") should be opt.get("optionId") — ACP spec field name is optionId, not id. Works today only if agents happen to send both. Will silently fall through to fallback if an agent only sends optionId.

• Extract selection logic into a standalone function — The priority loop is copy-pasted between connection.rs and the test. Extract to e.g. fn pick_best_option(options: &[Value]) -> Option<String> so both call the same code.

• Return cancelled instead of hardcoded fallback when all options are reject — Currently falls back to "allow_always" string which may not exist in the options array. acpx returns {outcome: {outcome: "cancelled"}} in this case, which is safer.

• Move qwen_acp_smoke_test to a separate PR — Unrelated to the permission fix scope.

• (Optional) Align preference order with acpx — openab prefers allow_always > allow_once, acpx prefers allow_once > allow_always. Not a blocker, but worth considering for ecosystem consistency.

@chaodu-agent, thank you very much for your detailed feedback and guidance. I have carefully addressed all the requirements and suggestions you provided. Could you please take another look at this PR when you have a moment? I truly appreciate your time and support. Thank you!

@chaodu-agent Both issues from your review have been addressed — selection now matches on kind instead of id, and reject options are properly excluded. Ready for re-review.

@chaodu-agent Follow-up on the optional priority-order nit: I intentionally kept allow_always > allow_once in openab. The reason is that openab acts as an auto-approve broker, so preferring the most permissive valid option minimizes repeated permission prompts and reduces the chance that the broker gets stuck in long-running sessions.

I did consider aligning with acpx's allow_once > allow_always, but for openab's role the tradeoff is different, so I kept the current order unless you want strict ecosystem parity here.

Also correcting my earlier PR comments for accuracy: this PR addresses #130, #111, and #241. The fallback behavior is allow_always > allow_once > first non-reject option, with cancelled when all provided options are reject-only.